Skip to content

fix: bound JDTLS startup waits - #1790

Closed
TyceHerrman wants to merge 2 commits into
oraios:mainfrom
TyceHerrman:agent/jdtls-startup-timeout
Closed

fix: bound JDTLS startup waits#1790
TyceHerrman wants to merge 2 commits into
oraios:mainfrom
TyceHerrman:agent/jdtls-startup-timeout

Conversation

@TyceHerrman

@TyceHerrman TyceHerrman commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Summary

  • apply one total deadline across JDTLS initialization, IntelliCode command registration/enablement, and ServiceReady
  • cap that deadline below Serena's outer tool timeout, reserving time to stop JDTLS and propagate the diagnostic error
  • track the current startup phase and latest language/status update
  • terminate the language-server process before raising a diagnostic timeout error
  • add and document the configurable ls_specific_settings.java.startup_timeout maximum (600 seconds total by default)

Why

The Windows JVM job in #1761 exhausted its 60-minute job limit with no indication of which startup wait had stalled. JDTLS can legitimately take a long time to initialize, but Serena's two required startup waits were unbounded.

A controlled watchdog canary captured Python threads, the process tree, a JDTLS JVM thread dump, phase/status history, LSP metadata, and the Eclipse workspace log. The canary showed healthy JDTLS initialization before the deliberate Python-side stall. The original test boundary was then exercised 53 times on Windows without reproducing a natural stall, so that occurrence did not identify an actionable upstream JDTLS or m2e defect.

A later natural recurrence occurred in #1811: Windows JVM run 30929370577, job 92059856781 again stopped at test_find_symbol_references[java_model_refs] until the 60-minute job limit. Its finalized log still cannot identify the stalled startup phase, which strengthens the case for bounded, diagnostic Serena-side waits but still does not justify attributing the underlying stall to a specific upstream component.

The first rebased #1790 run exposed an additional timeout-ordering issue in catch-all Ubuntu job 92087683238: the test dynamically activated a Java project, then Serena's 600-second outer tool timeout won the race against the original 600-second per-signal JDTLS timeout. The implementation now uses one total startup deadline, caps it using the existing language-server request budget, and reserves both JDTLS shutdown time and Serena's existing five-second error-propagation margin. This ensures the phase-rich JDTLS error surfaces before the generic tool timeout.

Fixes #1789. This supersedes the experimental diagnostics draft in #1786. The JDTLS workspace-cache isolation change remains separate in #1788.

Validation

Using Python 3.13.14:

  • uv run --no-sync pytest -q test/solidlsp/java/test_jdtls_startup_timeout.py test/solidlsp/java/test_jdtls_path_resolution.py (83 passed)
  • regression coverage runs a deliberately stalled JDTLS wait through Tool.apply_ex and verifies the detailed phase/status error wins before the generic outer timeout
  • uv run --no-sync poe lint
  • uv run --no-sync poe type-check
  • uv run --no-sync poe doc-build
  • git diff --check

@TyceHerrman

Copy link
Copy Markdown
Contributor Author

Windows JVM CI did not stall in JDTLS on this PR.

The finalized log shows:

  • test_find_symbol_references[java_model_refs] passed at 04:38:13 UTC
  • all later Java basic and diagnostics tests passed, through 04:40:28 UTC
  • output then stopped on TestKotlinDiagnostics::test_file_diagnostics[kotlin]
  • GitHub cancelled the job at the 60-minute cap and killed an orphan java process

This is a separate Kotlin test-quarantine gap. Kotlin is intentionally disabled on CI in test/conftest.py, and TestKotlinLanguageServer carries the matching skipif, but TestKotlinDiagnostics does not. The four preceding Kotlin basic tests were skipped; the diagnostics test was then allowed to start the quarantined IntelliJ-based server and consumed the remainder of the job.

I am leaving that unrelated fix out of this focused JDTLS PR. The Java evidence supports the existing conclusion: this run does not identify an upstream JDTLS/m2e defect.

@TyceHerrman
TyceHerrman force-pushed the agent/jdtls-startup-timeout branch from a4f2f40 to 23263a1 Compare August 4, 2026 18:13
@TyceHerrman
TyceHerrman marked this pull request as ready for review August 4, 2026 20:35
@opcode81

opcode81 commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Closing — rationale below

Thanks for your PR, but the PR's own premise undermines it: two occurrences ever, both Windows CI, same test, 0/53 reproductions, no stalled phase identified, no upstream defect found. That's not a diagnosed problem — that's an undiagnosed CI flake, and this is 245 lines of production hardening against it.

Core objections:

  1. No established problem. JDTLS stalling is not a pattern in real use. The likelier cause is environmental (runner starvation, shared workspace cache) — ci: exclude JDTLS workspaces from language-server cache #1788 addresses the latter directly. We should observe whether the flake survives ci: exclude JDTLS workspaces from language-server cache #1788 before touching the startup path.

  2. Regression risk. Current behavior waits indefinitely and succeeds on slow-but-healthy startups (first-run Gradle/Maven import, cold indexes, weak machines). This PR kills JDTLS at ~590 s, converting those into hard failures. That's a worse trade than the twice-observed hang.

  3. The tool-timeout capping is wrong on its own terms. The primary startup path runs as a background task with no timeout, and the motivating CI hangs happened in pytest fixtures where tool timeouts don't exist. Where startup does happen inside a tool call (LS auto-restart, terminated-LS retry), it begins mid-call, so capping against request_timeout — a per-request budget — guarantees no ordering. The Ubuntu "race" is a task-executor queuing race; the cap doesn't address it. It also hardcodes Serena's tool_timeout - 5 convention into solidlsp, which has no business knowing it, and adds a request_timeout property that's deliberately stale during startup.

  4. Cost. A permanent config option, base-class API surface, and 218 lines of tests built on object.__new__ against private helpers — for a hypothesis.

Closing. If future evidence identifies an actual stall phase, a minimal bounded-startup change can be considered on that basis.

@opcode81 opcode81 closed this Aug 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

JDTLS startup waits are unbounded and CI cannot identify the stalled phase

2 participants